Skip to content

fix: "verified up to" bar no longer gets stuck (minor refactor)#336

Merged
pimotte merged 4 commits intomappingfrom
verified-up-to-fix-no-refactor
Apr 14, 2026
Merged

fix: "verified up to" bar no longer gets stuck (minor refactor)#336
pimotte merged 4 commits intomappingfrom
verified-up-to-fix-no-refactor

Conversation

@GitLuckier
Copy link
Copy Markdown

@GitLuckier GitLuckier commented Mar 30, 2026

Description

Fixed the bug where the progress bar would get stuck, even if lean was done checking. This time a minimal fix without the refactoring.

The fix was to ensure the progress array is non empty when you try to access progress[0]:

		case MessageType.progress:
			{
				const { numberOfLines, progress } = msg.body;
				if (progress.length === 0) {
					editor.removeBusyIndicators();
					break;
				}

				const at = progress[0].range.start.line + 1;
				if (at === numberOfLines) {
					editor.reportProgress(at, numberOfLines, "File verified");
				} else {
					editor.reportProgress(at, numberOfLines, `Verified file up to line: ${at}`);
				}
				break;
			}

Instead of the major refactoring I did earlier, I could extract the messaging logic into its own file.

@GitLuckier GitLuckier marked this pull request as ready for review April 7, 2026 08:44
Copy link
Copy Markdown
Contributor

@pimotte pimotte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's actually what we really want! The logic in the listener is not too fancy, but clearly we can have bugs here, so I think it makes sense to extract the entire listener to a separate file and unit test them, in this PR.

@GitLuckier GitLuckier changed the title fix: "verified up to" bar no longer gets stuck (no refactor) fix: "verified up to" bar no longer gets stuck (minor refactor) Apr 13, 2026
@GitLuckier
Copy link
Copy Markdown
Author

I extracted all the messages send from waterproof-vscode to the editor into a file with a pure function. This should improve readability and allow us to write tests. I already wrote some tests for the non-trivial cases. This should catch bugs like this one in future.

@GitLuckier GitLuckier requested a review from pimotte April 13, 2026 17:29
Copy link
Copy Markdown
Contributor

@pimotte pimotte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, tested locally, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants